Skip to content

replace Bundle with the new AbstractBundle class #1514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 9 commits into from
Sep 28, 2024
Merged

replace Bundle with the new AbstractBundle class #1514

merged 9 commits into from
Sep 28, 2024

Conversation

@jrushlow
Copy link
Collaborator

@jrushlow jrushlow commented Apr 17, 2024 •
edited
Loading

Move the bundle from the old "Bundle" to the new "AbstractBundle" system following the best practices outlines in https://symfony.com/doc/current/bundles/best_practices.html

Nothing to see here yet...

see - symfony/symfony-docs#19793

I'm not sure if we should do this yet... Maker has a "leg up" on the other symfony/* bundles because we're a bit bleeding edge. Our minimum symfony/* is 6.4 - but by merging this PR, we would be breaking away from say, FrameworkBundle that still uses the legacy Bundle class...

On the flip side, if we do press forward with this PR - we could reference this change somehow in the docs to show others how we "converted" to the new simplification strategy.

Pinging @nicolas-grekas @javiereguiluz & @yceruto This isn't review ready yet, but feedback on if we should press forward with this would be awesome.

Merge blockers:

  • src/DependencyInjection/CompilerPass/* figure out if these should live here still - or if there is a better way...
  • src/Resources/bin - This is our "bundled" php-cs-fixer, it needs a new home (not intended to be directly called by the user...) the bundled php-cs-fixer used by the template linter was replaced with a shim
  • src/Resources/config/php-cs-fixer.config.php - used by our internal php-cs-fixer when running make:* - make sure this path isnt hard coded.. If it is, fix it to use the new config/php-cs-fixer.config.php path.
  • src/Resources/doc -> docs/
  • Move help files into new directory structure  #1605 src/Resources/help - figure out a home for these files - the AbstractMaker? (or another parent class) needs to be updated to point the command to the new help location This will done in a separate PR as it will create a messy diff...
  • move templates to root template dir #1606 src/Resources/Skeleton - our "templates" need a new home. These feel like they should live in src maybe something like src/Skeleton? hmm.... This will done in a separate PR as it will create a messy diff...
  • Test, test in ci, test in linked apps, test some more, run some tests....

@jrushlow jrushlow added the Minor Minor Enhancement label Apr 17, 2024
@jrushlow jrushlow force-pushed the minor/new-bundle-structure branch from b5544df to 9e2c829 Compare April 17, 2024 10:16
@jrushlow jrushlow added the Status: Needs Work Additional work is needed label Apr 17, 2024
@yceruto
Copy link
Member

yceruto commented Apr 17, 2024

src/DependencyInjection/CompilerPass/* figure out if these should live here still - or if there is a better way...

It's still the correct place for them in my opinion.

src/Resources/bin - This is our "bundled" php-cs-fixer, it needs a new home (not intended to be directly called by the user...)

It should be <root>/bin/ instead (according best practices document)

src/Resources/config/php-cs-fixer.config.php - used by our internal php-cs-fixer when running make:* - make sure this path isnt hard coded.. If it is, fix it to use the new config/php-cs-fixer.config.php path.

I usually put this config file at the root of the project, but it'd be okay under <root>/config/ as well

src/Resources/doc -> doc/

<root>/docs/ in plural (according to best practices doc)

src/Resources/help - figure out a home for these files - the AbstractMaker? (or another parent class) needs to be updated to point the command to the new help location

They look like configuration files to me, as such they could be moved to <root>/config/help/ dir

src/Resources/Skeleton - our "templates" need a new home. These feel like they should live in src maybe something like src/Skeleton? hmm....

Kind of templates! <root>/skeleton/ lower case 👍

@jrushlow jrushlow force-pushed the minor/new-bundle-structure branch from 10d1ba8 to 9cde4be Compare September 27, 2024 05:42
@jrushlow jrushlow added this to the v1.62.0 milestone Sep 27, 2024
@jrushlow jrushlow changed the title wip - use the new bundle structure replace Bundle with the new AbstractBundle class Sep 27, 2024
@jrushlow jrushlow added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Sep 27, 2024
This was referenced Sep 27, 2024
Merged
Merged
@jrushlow jrushlow merged commit 673c3a5 into symfony:main Sep 28, 2024
@jrushlow jrushlow deleted the minor/new-bundle-structure branch September 28, 2024 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Minor Minor Enhancement Status: Needs Review Needs to be reviewed

Projects

None yet

Milestone

v1.62.0

Development

Successfully merging this pull request may close these issues.

2 participants

@jrushlow @yceruto